Add WebSocket handler for Browser and Node #9191
Conversation
|
There was a problem hiding this comment.
See the comments for more but I'm leaning towards not offering this for Node given the complications and not being sure there's a big demand for it anyway. I feel really bad about that given the effort you've put into the Node implementation and the clever dead code trick but it might come back.
Size Report 1Affected ProductsNo changes between base commit (a4897a6) and merge commit (a06a3a1).Test Logs |
Size Analysis Report 1Affected ProductsNo changes between base commit (a4897a6) and merge commit (a06a3a1).Test Logs |
| @@ -159,6 +159,7 @@ | |||
| "typescript": "5.5.4", | |||
| "watch": "1.0.2", | |||
| "webpack": "5.98.0", | |||
| "ws": "8.18.3", | |||
There was a problem hiding this comment.
We're getting rid of this, right?
Edit: Oh, you need it for the server for the test.
Edit 2: Should probably leave it in the package.json for the product package if it's not used anywhere else.
| * limitations under the License. | ||
| */ | ||
|
|
||
| import { NodeWebSocketHandler } from './node/websocket'; |
There was a problem hiding this comment.
Does this work? The rollup alias just replaces the node in the import path, it doesn't replace the name of the imported symbol, right? Isn't this supposed to be just WebSocketHandlerImpl or something and the exported symbol from node/websocket.ts and browser/websocket.ts both have the same name?
Adds a WebSocket handler with implementations in Browser and Node environments. This will be used for BiDi.
Changes:
WebSocketHandler:BrowserWebSocketHandler) and Node (NodeWebSocketHandler).NodeWebSocketHandleruses the built-in globalWebSocketavailable in Node >= 22.wsas a dev dependency for types and the mock test server (Node 22 doesn't support web socket servers).BrowserWebSocketHandlerusing a mockedwindow.WebSocket.NodeWebSocketHandlerusing a localMockWebSocketServer.test:nodegenerateAliasConfigto the set of rollup helpers, and uses it for platform import aliasing.